Skip to content

Conversation

@jreineckearm
Copy link
Collaborator

Fixes

N/A

Changes

  • Change TPIP workflow to run as part of PRs rather than post commit to main. This shall work around the CI user not having (and not supposed to have) direct write access to main.

Screenshots

Checklist

@jreineckearm jreineckearm changed the base branch from disable-tpip-generation to main March 3, 2025 10:09
@JonatanAntoni
Copy link
Member

${{ github.ref_name }} doesn't work for PRs. We need to change that to ${{ github.head_ref }}, I think.

@jreineckearm
Copy link
Collaborator Author

${{ github.ref_name }} doesn't work for PRs. We need to change that to ${{ github.head_ref }}, I think.

Thanks! Just compared to other workflows. Removing ref altogether should also fix it. It defaults to the event ref/SHA, i.e. the used branch.

@JonatanAntoni
Copy link
Member

${{ github.ref_name }} doesn't work for PRs. We need to change that to ${{ github.head_ref }}, I think.

Thanks! Just compared to other workflows. Removing ref altogether should also fix it. It defaults to the event ref/SHA, i.e. the used branch.

It won't be able to commit the report into the PR branch if relying on the default SHA (which typically is a merge of pr head and base branches.

@jreineckearm
Copy link
Collaborator Author

It won't be able to commit the report into the PR branch if relying on the default SHA (which typically is a merge of pr head and base branches.

Thanks! Should have listened. It indeed can't operate on the default.

Copy link
Member

@JonatanAntoni JonatanAntoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, its looking good and seems to work as expected.
I.e., the TPIP.md got updated.

@jreineckearm
Copy link
Collaborator Author

Yes, it only updated the timestamp. Let's see how this behave on branches without TPIP updates.
Was briefly concerned about TPIP and CI running in parallel. But that only affects the VSIX packages attached to the branch actions. I.e. all should come together properly on main.
Can probably add a dependency in a later step, was mainly after fixing post-commit action failures.

@jreineckearm
Copy link
Collaborator Author

Having written that, something seems to confuse now the other workflows. Presumably the TPIP commit while the other workflows were running. Need to have another look in the afternoon.

@jreineckearm
Copy link
Collaborator Author

After discussing again with Jonatan, I looks like this is a limitation we have to live with for now. We can review after Embedded World how to further smoothen this process. The only downside is that we worst case have to fiddle around with PRs that change dependencies.

@jreineckearm jreineckearm merged commit c1c9e94 into main Mar 3, 2025
@jreineckearm jreineckearm deleted the tpip-on-pr branch March 3, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants